Skip to content

feat: Add act #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 11, 2019
Merged

feat: Add act #41

merged 2 commits into from
Aug 11, 2019

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Jul 30, 2019

Adds a function which can wrap testing actions and guarantees that after awaited state updates are flushed to the DOM.

Internal it just calls the callback and returns the promise from svelte's tick. This should replace the current documented approach of using wait + expect or waitForElement. The previous behavior either waited too long (default timeout of 2s) or was flaky if the timeout was really short. It's also slow when using wait since that uses polling.

It essentially mimics the act from React.

@eps1lon eps1lon added the enhancement New feature or request label Jul 30, 2019
@eps1lon eps1lon marked this pull request as ready for review July 30, 2019 09:15
@eps1lon eps1lon requested a review from benmonro July 30, 2019 09:15
@benmonro benmonro requested a review from EmilTholin July 30, 2019 20:10
@benmonro
Copy link
Member

LGTM, thanks for contributing, @EmilTholin look good to you?

Copy link
Member

@benmonro benmonro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small thing then we can merge

@EmilTholin
Copy link
Contributor

EmilTholin commented Aug 5, 2019

Sorry for the slow reply, I just got back from vacation.

I think it looks good! I think an await anywhere in the test will make it so the following statements are run in the next Svelte tick, but it could be nice to have act to make this library feel more familiar to react-testing-library users.

@benmonro
Copy link
Member

benmonro commented Aug 5, 2019

@EmilTholin if that's the case then I'm actually thinking it would stay as is. I'd rather keep this in the spirit of svelte. Can you confirm an await will work? If so maybe we busy update the docs accordingly?

@EmilTholin
Copy link
Contributor

This was discussed in this issue a while back and @pngwn was kind enough to outline and explain why await anywhere might be enough.

The ability to return any promise from act might warrant adding it to this library as well though, but I'm not sure.

@eps1lon
Copy link
Member Author

eps1lon commented Aug 5, 2019

await anywhere might be enough.

I'd rather use public API from svelte. Looking at

Returns a promise that resolves once any pending state changes have been applied, or in the next microtask if there are none.

it sounds like await is only enough when there are no pending state updates. Once there are some you are out of luck. Let's not be too clever about this. Especially async stuff.

@eps1lon eps1lon requested a review from benmonro August 5, 2019 17:58
@benmonro benmonro merged commit d26a497 into testing-library:master Aug 11, 2019
@benmonro
Copy link
Member

@allcontributors please add @eps1lon for code

@allcontributors
Copy link
Contributor

@benmonro

I've put up a pull request to add @eps1lon! 🎉

@benmonro
Copy link
Member

🎉 This PR is included in version 1.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants